Closed
Bug 1053758
Opened 10 years ago
Closed 10 years ago
Update "add to home screen" dialog to meet new interaction and UX specs
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(b2g-v2.1 fixed, b2g-v2.2 fixed)
RESOLVED
FIXED
2.1 S4 (12sep)
People
(Reporter: benfrancis, Assigned: crdlc)
References
Details
(Keywords: late-l10n, polish, Whiteboard: [systemsfe])
Attachments
(3 files, 4 obsolete files)
190 bytes,
text/html
|
kgrandon
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details |
39.21 KB,
image/png
|
epang
:
ui-review+
|
Details |
33.16 KB,
image/png
|
epang
:
ui-review+
|
Details |
Interaction spec: https://mozilla.app.box.com/s/lbw2wzw3p4jvxs24k4sg/1/2099951272/19877711201/1 (page 26) Visual spec: https://mozilla.app.box.com/s/wj69q6eqgbxb5i8yogqo
Updated•10 years ago
|
Summary: Update "add to homescreen" dialog to meet new interaction and UX specs → Update "add to home screen" dialog to meet new interaction and UX specs
Reporter | ||
Updated•10 years ago
|
No longer blocks: browser-chrome-mvp
Comment 1•10 years ago
|
||
Peter, Eric - I'm assuming that we use the same dialog for add and edit cases, right? Currently we have the url field there as well and it's editable, are we making a conscious decision to remove it?
Flags: needinfo?(pdolanjski)
Flags: needinfo?(epang)
Comment 2•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #1) > Peter, Eric - I'm assuming that we use the same dialog for add and edit > cases, right? Currently we have the url field there as well and it's > editable, are we making a conscious decision to remove it? Good question. +Francis since he didn't have the url field in the interaction spec either.
Flags: needinfo?(pdolanjski) → needinfo?(fdjabri)
Updated•10 years ago
|
Comment 3•10 years ago
|
||
Yes, this was a conscious decision. I don't really feel that the URL field was necessary so the design felt simpler without it.
Flags: needinfo?(fdjabri)
Flags: needinfo?(epang)
Comment 4•10 years ago
|
||
Cristian - would you have some cycles to help us with this dialog UX? Thanks!
Flags: needinfo?(crdlc)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Flags: needinfo?(crdlc)
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
Francis, I would like to show you the screenshots (add and edit UI) in order to know the correct titles, to confirm that url field disappears, etc. Thanks a lot
Flags: needinfo?(fdjabri)
Assignee | ||
Comment 10•10 years ago
|
||
Reporter | ||
Comment 11•10 years ago
|
||
(In reply to Francis Djabri [:djabber] from comment #3) > Yes, this was a conscious decision. I don't really feel that the URL field > was necessary so the design felt simpler without it. Not being able to edit the URL of a bookmark seems like a bit of a pain and a feature we'd be losing from 2.0 :(
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8482236 [details]
Add link
I know that we need the feedback from Francis for the title but I think that the UI is ready to review
Attachment #8482236 -
Flags: ui-review?(epang)
Assignee | ||
Updated•10 years ago
|
Attachment #8482242 -
Attachment description: WIP → Go to PR
Attachment #8482242 -
Flags: review?(kgrandon)
Comment 13•10 years ago
|
||
Comment on attachment 8482242 [details]
Go to PR
I think it's close, and the code looks good, but it appears there's a perma-failing marionette test. Can you fix browser_chrome_bookmark_web_result_test.js and re-flag me for review? Thanks!
Attachment #8482242 -
Flags: review?(kgrandon)
Comment 14•10 years ago
|
||
The visuals seem to follow Eric's spec, but flagging Eric to confirm. As for the title, this should be "Add to Homescreen" according to the spec.
Flags: needinfo?(fdjabri)
Updated•10 years ago
|
Flags: needinfo?(epang)
Comment 15•10 years ago
|
||
(In reply to Francis Djabri [:djabber] from comment #14) > The visuals seem to follow Eric's spec, but flagging Eric to confirm. > As for the title, this should be "Add to Homescreen" according to the spec. Hey Francis - We've actually gotten feedback that we should display "Add to home screen" everywhere from l10n/copy folks. In fact, I suppose "homescreen" isn't a real word and we were requested to use "home screen" in all places instead, specifically lower cased and two words. If you do find yourself in the spec again soon - can you update these references?
Flags: needinfo?(fdjabri)
Assignee | ||
Comment 16•10 years ago
|
||
But what should we show for editing bookmarks? https://bug1053758.bugzilla.mozilla.org/attachment.cgi?id=8482238 "Edit in home screen"?
Assignee | ||
Updated•10 years ago
|
Attachment #8482236 -
Attachment is obsolete: true
Attachment #8482236 -
Flags: ui-review?(epang)
Assignee | ||
Updated•10 years ago
|
Attachment #8482237 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8482238 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8482239 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
Assignee | ||
Comment 17•10 years ago
|
||
Attachment #8483294 -
Flags: ui-review?(epang)
Assignee | ||
Comment 18•10 years ago
|
||
Attachment #8483295 -
Flags: ui-review?(epang)
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8482242 [details]
Go to PR
Fixed test Kevin
Attachment #8482242 -
Flags: review?(kgrandon)
Assignee | ||
Comment 20•10 years ago
|
||
"Rename bookmark", "Rename in home screen",... I need this info before merging, thx (In reply to Cristian Rodriguez (:crdlc) from comment #16) > But what should we show for editing bookmarks? > https://bug1053758.bugzilla.mozilla.org/attachment.cgi?id=8482238 "Edit in > home screen"?
Comment 21•10 years ago
|
||
I think keeping either "Edit link" or "Edit bookmark" would make the most sense here.
Comment 22•10 years ago
|
||
Comment on attachment 8482242 [details]
Go to PR
Let's make sure we hear from UX on the text before landing - thanks!
Attachment #8482242 -
Flags: review?(kgrandon) → review+
Comment 23•10 years ago
|
||
Comment on attachment 8483295 [details]
Edit bookmark
Looking at the screen shots this looks good to me! I think 'Edit bookmark'. Francis do you agree?
Attachment #8483295 -
Flags: ui-review?(epang) → ui-review+
Flags: needinfo?(epang)
Comment 24•10 years ago
|
||
Comment on attachment 8483294 [details]
Add to home screen
Visually looks to spec based on the screen shot. When flashing the patch I wasn't able to edit the input field and none of the buttons worked, but I'm guessing it's probably my problem. Thanks!
Attachment #8483294 -
Flags: ui-review?(epang) → ui-review+
Updated•10 years ago
|
Blocks: rocketbar-mvp
Updated•10 years ago
|
No longer blocks: browser-chrome-mvp
Assignee | ||
Comment 25•10 years ago
|
||
I have changed the string to "Edit bookmark" in the patch
Assignee | ||
Comment 26•10 years ago
|
||
Upps I merged without Francis' feedback :( Sorry, I didn't realize. Francis, if you are ok with "Edit bookmark" would be perfect, in another case, I could revert this patch and address your suggest for the string. Merged in master: https://github.com/mozilla-b2g/gaia/commit/c26c8a237bc665a9a7e55dd248ab8734fd7d9959
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 27•10 years ago
|
||
Sorry but this needs to be fixed (up to you if you prefer to backout or fix it in a follow-up). https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Changing_existing_strings Changing case is acceptable, changing strings like this no. -add-bookmark-header=Add link -edit-bookmark-header=Edit link +add-bookmark-header=Add to home screen +edit-bookmark-header=Edit bookmark
Flags: needinfo?(crdlc)
Comment 28•10 years ago
|
||
Sorry, I left my R+ before this change and didn't catch it. I think Cristian might be gone for the day, so let's backout for now and Cristian can re-land after updating the keys. Reverted: https://github.com/mozilla-b2g/gaia/commit/6eb0f8fee8d14c0868ad9d4b030dcf8bf90f0141
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 29•10 years ago
|
||
(In reply to Cristian Rodriguez (:crdlc) from comment #26) > Upps I merged without Francis' feedback :( Sorry, I didn't realize. Francis, > if you are ok with "Edit bookmark" would be perfect, in another case, I > could revert this patch and address your suggest for the string. > > Merged in master: > > https://github.com/mozilla-b2g/gaia/commit/ > c26c8a237bc665a9a7e55dd248ab8734fd7d9959 Hi Cristian, Edit bookmark is fine with me.
Updated•10 years ago
|
Flags: needinfo?(fdjabri)
Comment 30•10 years ago
|
||
Cristian - To clarify, I think you only need to update the l10n keys for the strings and how they're consumed here before landing. They just need to be changed so our l10n teams know they've been updated and can re-translate them. Thanks!
Assignee | ||
Comment 32•10 years ago
|
||
Merged in master: https://github.com/mozilla-b2g/gaia/commit/f818d4f9a6b7f5dd14ea7457220ea5764ff49a5a
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 33•10 years ago
|
||
Comment on attachment 8482242 [details]
Go to PR
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: users can see how the bookmark will be on the home screen because the icon is displayed while they are bookmarking so it does happier to end users thought
[Testing completed]: Added unit tests
[Risk to taking this patch] (and alternatives if risky): no alternatives
[String changes made]: yes
Attachment #8482242 -
Flags: approval-gaia-v2.1?
Updated•10 years ago
|
Attachment #8482242 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Comment 34•10 years ago
|
||
v2.1: https://github.com/mozilla-b2g/gaia/commit/1d5474a788240919b5d15a7f9e0c56d0dd051072
You need to log in
before you can comment on or make changes to this bug.
Description
•